fix: clicking on Pressable located in screen header #2466

coado · 2024-10-29T14:53:39Z

Description

This PR fixes clicking on Pressables that are added to the native header. Previously, Yoga had incorrect information about the location of the content in the header.

The first step was to remove top: "-100%" style from the ScreenStackHeaderConfig which made Yoga think, that the content is pushed up by the size of its parent (Screen size).

The second step involved updating layoutMetrics of the RNSScreenStackHeaderConfigShadowNode. The entire app content is pushed down by the size of the header, so it also has an impact on the header config in Yoga metrics. To mitigate this, the origin of RNSScreenStackHeaderConfigShadowNode is decreased by the size of the header which will zero out eventually leaving the header content in the desired position. On iOS this position is actually moved by the top inset size, so we also have to take it into account when setting header config layout metrics.

Fixes #2219

Changes

Updated ScreenShadowNode to decrease origin.y of the HeaderConfigShadowNode by the size of the header. Added paddingTop to HeaderConfigState and set it as origin offset on iOS.

Screenshots / GIFs

Before

iOS	Android
ios-before.mov	android-before.mov

After

iOs	Android
ios-after.mov	android-after.mov

Test code and steps to reproduce

Tested on this example:

code

import { NavigationContainer } from "@react-navigation/native";
import { createNativeStackNavigator, NativeStackNavigationProp } from "@react-navigation/native-stack";
import React, { ForwardedRef, forwardRef } from "react";
import { findNodeHandle, Pressable, PressableProps, StyleSheet, Text, View, } from "react-native";

type StackParamList = {
  Home: undefined,
}

type RouteProps = {
  navigation: NativeStackNavigationProp<StackParamList>;
}

type PressableState = 'pressed-in' | 'pressed' | 'pressed-out'


const Stack = createNativeStackNavigator<StackParamList>();


const PressableWithFeedback = forwardRef((props: PressableProps, ref: ForwardedRef<View>): React.JSX.Element => {
  const [pressedState, setPressedState] = React.useState<PressableState>('pressed-out');

  const onPressInCallback = React.useCallback((e) => {
    console.log('Pressable onPressIn', {
      locationX: e.nativeEvent.locationX,
      locationY: e.nativeEvent.locationY,
      pageX: e.nativeEvent.pageX,
      pageY: e.nativeEvent.pageY,
    });
    setPressedState('pressed-in');
    props.onPressIn?.();
  }, []);

  const onPressCallback = React.useCallback(() => {
    console.log('Pressable onPress');
    setPressedState('pressed');
  }, []);

  const onPressOutCallback = React.useCallback(() => {
    console.log('Pressable onPressOut');
    setPressedState('pressed-out');
  }, []);

  const onResponderMoveCallback = React.useCallback(() => {
    console.log('Pressable onResponderMove');
  }, []);

  const contentsStyle = pressedState === 'pressed-out'
    ? styles.pressablePressedOut
    : (pressedState === 'pressed'
      ? styles.pressablePressed
      : styles.pressablePressedIn);

  return (
    <View ref={ref} style={[contentsStyle, { width: "100%"}]}>
      <Pressable
        onPressIn={onPressInCallback}
        onPress={onPressCallback}
        onPressOut={onPressOutCallback}
        onResponderMove={onResponderMoveCallback}
      >
        {props.children}
      </Pressable>
    </View>

  );
})

function HeaderTitle(): React.JSX.Element {

  return (
    <PressableWithFeedback
      onPressIn={() => {
        console.log('Pressable onPressIn')
      }}
      onPress={() => console.log('Pressable onPress')}
      onPressOut={() => console.log('Pressable onPressOut')}
      onResponderMove={() => console.log('Pressable onResponderMove')}
      ref={ref => {
        console.log(findNodeHandle(ref));
        ref?.measure((x, y, width, height, pageX, pageY) => {
          console.log('header component measure', { x, y, width, height, pageX, pageY });
        })
      }}
    >
      <View style={{ height: 40, justifyContent: 'center', alignItems: 'center' }}>
        <Text style={{ alignItems: 'center' }}>Regular Pressable</Text>
      </View>
    </PressableWithFeedback>
  )
}

function Home(_: RouteProps): React.JSX.Element {
  return (
    <View style={{ flex: 1, backgroundColor: 'rgba(0, 0, 0, .8)' }}
    >
      <View style={{ flex: 1, alignItems: 'center', marginTop: 48 }}>
        <PressableWithFeedback
          onPressIn={() => console.log('Pressable onPressIn')}
          onPress={() => console.log('Pressable onPress')}
          onPressOut={() => console.log('Pressable onPressOut')}
        >
          <View style={{ height: 40, width: 200, justifyContent: 'center', alignItems: 'center' }}>
            <Text style={{ alignItems: 'center' }}>Regular Pressable</Text>
          </View>
        </PressableWithFeedback>
      </View>
    </View>
  );
}

function App(): React.JSX.Element {
  return (
    <NavigationContainer>
      <Stack.Navigator>
        <Stack.Screen
          name="Home"
          component={Home}
          options={{
            headerTitle: HeaderTitle,
          }}
        />
      </Stack.Navigator>
    </NavigationContainer>
  );
}

const styles = StyleSheet.create({
  pressablePressedIn: {
    backgroundColor: 'lightsalmon',
  },
  pressablePressed: {
    backgroundColor: 'crimson',
  },
  pressablePressedOut: {
    backgroundColor: 'lightseagreen',
  }
});


export default App;

…owNode

alduzy · 2024-10-30T10:51:13Z

Hi @coado I like this approach. However, I'm afraid in current form it may not respect the placement of the elements in regards to flex layout of the header. Have you tested it with smaller elements, headerLeft and/or headerRight for example?

You should be able to use the Element Inspector from the dev menu to inspect the actual placement of the pressables laid out by yoga. I remember using it in #2292

coado · 2024-10-31T10:43:36Z

Hi @coado I like this approach. However, I'm afraid in current form it may not respect the placement of the elements in regards to flex layout of the header. Have you tested it with smaller elements, headerLeft and/or headerRight for example?

You should be able to use the Element Inspector from the dev menu to inspect the actual placement of the pressables laid out by yoga. I remember using it in #2292

Hey @alduzy, thanks for the reply! This is how it looks when I set Pressable on headerRight only or on both:

right	both

Please let me know if this is desired behaviour.

Also I've checked a placement using inspector as you proposed and it seems like Pressable boundary in Yoga is not perfectly aligned with what is displayed.

This is something that I will have a closer look at!

code

import { NavigationContainer } from "@react-navigation/native";
import { createNativeStackNavigator, NativeStackNavigationProp } from "@react-navigation/native-stack";
import React, { ForwardedRef, forwardRef } from "react";
import { findNodeHandle, Pressable, PressableProps, StyleSheet, Text, View, Button } from "react-native";

type StackParamList = {
  Home: undefined,
}

type RouteProps = {
  navigation: NativeStackNavigationProp<StackParamList>;
}

type PressableState = 'pressed-in' | 'pressed' | 'pressed-out'


const Stack = createNativeStackNavigator<StackParamList>();


const PressableWithFeedback = forwardRef((props: PressableProps, ref: ForwardedRef<View>): React.JSX.Element => {
  const [pressedState, setPressedState] = React.useState<PressableState>('pressed-out');

  const onPressInCallback = React.useCallback((e) => {
    console.log('Pressable onPressIn', {
      locationX: e.nativeEvent.locationX,
      locationY: e.nativeEvent.locationY,
      pageX: e.nativeEvent.pageX,
      pageY: e.nativeEvent.pageY,
    });
    setPressedState('pressed-in');
    props.onPressIn?.();
  }, []);

  const onPressCallback = React.useCallback(() => {
    console.log('Pressable onPress');
    setPressedState('pressed');
  }, []);

  const onPressOutCallback = React.useCallback(() => {
    console.log('Pressable onPressOut');
    setPressedState('pressed-out');
  }, []);

  const onResponderMoveCallback = React.useCallback(() => {
    console.log('Pressable onResponderMove');
  }, []);

  const contentsStyle = pressedState === 'pressed-out'
    ? styles.pressablePressedOut
    : (pressedState === 'pressed'
      ? styles.pressablePressed
      : styles.pressablePressedIn);

  return (
    <View ref={ref} style={[contentsStyle]}>
      <Pressable
        onPressIn={onPressInCallback}
        onPress={onPressCallback}
        onPressOut={onPressOutCallback}
        onResponderMove={onResponderMoveCallback}
      >
        {props.children}
      </Pressable>
    </View>

  );
})

function HeaderTitle(): React.JSX.Element {

  return (
    <PressableWithFeedback
      onPressIn={() => {
        console.log('Pressable onPressIn')
      }}
      onPress={() => console.log('Pressable onPress')}
      onPressOut={() => console.log('Pressable onPressOut')}
      onResponderMove={() => console.log('Pressable onResponderMove')}
      ref={ref => {
        console.log(findNodeHandle(ref));
        ref?.measure((x, y, width, height, pageX, pageY) => {
          console.log('header component measure', { x, y, width, height, pageX, pageY });
        })
      }}
    >
      <View style={{ height: 40, justifyContent: 'center', alignItems: 'center' }}>
        <Text style={{ alignItems: 'center' }}>Regular Pressable</Text>
      </View>
    </PressableWithFeedback>
  )
}

function Home(_: RouteProps): React.JSX.Element {
  return (
    <View style={{ flex: 1, backgroundColor: 'rgba(0, 0, 0, .8)' }}
    >
      <View style={{ flex: 1, alignItems: 'center', marginTop: 48 }}>
        <PressableWithFeedback
          onPressIn={() => console.log('Pressable onPressIn')}
          onPress={() => console.log('Pressable onPress')}
          onPressOut={() => console.log('Pressable onPressOut')}
        >
          <View style={{ height: 40, width: 200, justifyContent: 'center', alignItems: 'center' }}>
            <Text style={{ alignItems: 'center' }}>Regular Pressable</Text>
          </View>
        </PressableWithFeedback>
      </View>
    </View>
  );
}

function App(): React.JSX.Element {
  return (
    <NavigationContainer>
      <Stack.Navigator>
        <Stack.Screen
          name="Home"
          component={Home}
          options={{
            // headerTitle: HeaderTitle,
            headerRight: HeaderTitle,
            // headerLeft: HeaderTitle
          }}
        />
      </Stack.Navigator>
    </NavigationContainer>
  );
}

const styles = StyleSheet.create({
  pressablePressedIn: {
    backgroundColor: 'lightsalmon',
  },
  pressablePressed: {
    backgroundColor: 'crimson',
  },
  pressablePressedOut: {
    backgroundColor: 'lightseagreen',
  }
});


export default App;

coado · 2024-10-31T10:49:14Z

Actually, when the Pressable is set as the headerRight only it doesn't work 🤔

jiroscripts · 2024-11-12T14:14:27Z

Hello @coado

Any news on this issue?

I started upgrading my project to RN 0.76 and this issue is by far the most problematic

coado · 2024-11-13T09:04:46Z

Hey @thibaultcapelli
Sorry for the delay. I will get back to this issue shortly.

kkafar · 2024-11-13T09:15:23Z

I was just looking through the code and determined that the only reliable solution would be to update position of header elements in ShadowTree (ST) based on their position in HostTree (HT), i. e. you do send additional information on topinset now - maybe let's send whole frame instead and update headersubviews layout metrics in shadow node? I think this is only way to get this at least partially consistent.

acrabb · 2024-11-18T17:04:59Z

+1 for this issue 🙏

jiroscripts · 2024-12-03T09:35:19Z

Please, merge this

jiroscripts · 2024-12-18T09:39:44Z

@kkafar What are we waiting for ?

I cannot understand there is new release candidates and this several issue is just waiting for weeks.

Native stack looks actually the way to go but there is no way to use it with a fresh RN project if you use a pressable in header which is a pretty common thing.

kkafar · 2024-12-20T09:44:47Z

This will land with 4.5.0 in the early January alongside support for 0.77. I know this fix is crucial, however it still requires more testing as it affects almost every native header behaviour and multiple regressions are possible.

For this 1-2 week period please use the patch: https://patch-diff.githubusercontent.com/raw/software-mansion/react-native-screens/pull/2466.diff (with patch-package). The more feedback I get on whether this PR is reliable the earlier I'll be able to release.

sadikyalcin · 2024-12-20T10:04:42Z

This will land with 4.5.0 in the early January alongside support for 0.77. I know this fix is crucial, however it still requires more testing as it affects almost every native header behaviour and multiple regressions are possible.

For this 1-2 week period please use the patch: https://patch-diff.githubusercontent.com/raw/software-mansion/react-native-screens/pull/2466.diff (with patch-package). The more feedback I get on whether this PR is reliable the earlier I'll be able to release.

Not sure if this was handled / mentioned but software-mansion/react-native-screens#@dmalecki/pressable-in-header crashes on Android with ViewManagerResolver returned null for either RNSScreenContentWrapper or RCTRNSScreenContentWrapper error.

kkafar · 2024-12-20T10:34:06Z

@sadikyalcin is this something you get specifically with this PR? It looks like none of the library components are registered and this PR should not impact this behaviour. Please verify that node_modules/react-native-screens/android/ is not empty and there are source files there

sadikyalcin · 2024-12-20T10:51:29Z

@sadikyalcin is this something you get specifically with this PR? It looks like none of the library components are registered and this PR should not impact this behaviour. Please verify that node_modules/react-native-screens/android/ is not empty and there are source files there

Yep with that PR only. The diff file you provided is okay. I've got a build that is working at the moment so don't want to switch back to that PR for now as I got other things to sort out but I will re-check that PR again later.

Also, the header buttons seems to be working reliably too.

nikodunk · 2024-12-22T00:06:46Z

Sorry to be dense, but I'm failing to test this PR due to dependency conflicts on a fully-updated version of an expo app, with this error. Not blocking for me, but I just can't help test.

from package.json:

...
"react-native-screens": "software-mansion/react-native-screens#@dmalecki/pressable-in-header",
...

error

npm error code 1
npm error git dep preparation failed
npm error command /var/home/niko/.nvm/versions/node/v22.8.0/bin/node /var/home/niko/.nvm/versions/node/v22.8.0/lib/node_modules/npm/bin/npm-cli.js install --force --cache=/var/home/niko/.npm --prefer-offline=false --prefer-online=false --offline=false --no-progress --no-save --no-audit --include=dev --include=peer --include=optional --no-package-lock-only --no-dry-run
npm error > [email protected] prepare
npm error > bob build && husky install
npm error
npm error ℹ Building target commonjs
npm error ℹ Cleaning up previous build at lib/commonjs
npm error ℹ Compiling 73 files in src with babel
npm error ✔ Wrote files to lib/commonjs
npm error ℹ Building target module
npm error ℹ Cleaning up previous build at lib/module
npm error ℹ Compiling 73 files in src with babel
npm error ✔ Wrote files to lib/module
npm error ℹ Building target typescript
npm error ℹ Cleaning up previous build at lib/typescript
npm error ℹ Generating type definitions with tsc
npm error src/gesture-handler/defaults.ts:7:14 - error TS2322: Type '{ absoluteX: number; absoluteY: number; handlerTag: number; numberOfPointers: number; state: 0; translationX: number; translationY: number; velocityX: number; velocityY: number; x: number; y: number; }' is not assignable to type 'GestureUpdateEvent<PanGestureHandlerEventPayload>'.
npm error   Property 'pointerType' is missing in type '{ absoluteX: number; absoluteY: number; handlerTag: number; numberOfPointers: number; state: 0; translationX: number; translationY: number; velocityX: number; velocityY: number; x: number; y: number; }' but required in type 'GestureEventPayload'.
npm error
npm error 7 export const DefaultEvent: GestureUpdateEvent<PanGestureHandlerEventPayload> = {
npm error                ~~~~~~~~~~~~
npm error
npm error   node_modules/react-native-gesture-handler/lib/typescript/handlers/gestureHandlerCommon.d.ts:12:5
npm error     12     pointerType: PointerType;
npm error            ~~~~~~~~~~~
npm error     'pointerType' is declared here.
npm error
npm error
npm error Found 1 error in src/gesture-handler/defaults.ts:7
npm error
npm error ✖ Failed to build definition files.

I've matched the react-native-gesture-handler version I believe you're using.

workaround from software-mansion/react-native-screens#2219 (comment) software-mansion/react-native-screens#2466

satya164 · 2024-12-24T13:10:29Z

@nikodunk alternatively you can clone this repo, switch to the branch, run yarn and then npm pack which will produce a tar.gz file - and you can do npm install /path/to/tar.gz

coado added 3 commits October 29, 2024 14:34

remove header offset from RNSScreenStackHeaderConfigShadowNode

0044582

remove top: '-100%' from ScreenStackHeaderConfig

fa6ba8c

rename topInsetOffset to paddingTop in RNSScreenStackHeaderConfigShad…

47212d4

…owNode

kkafar self-requested a review October 30, 2024 11:51

jiroscripts mentioned this pull request Nov 12, 2024

Pressable elements not working in native-stack on Android devices with new architecture #2219

Open

coado added 6 commits November 13, 2024 13:34

Merge branch 'main' into @dmalecki/pressable-in-header

7aa8889

header config frame

0d80a85

header config state

8047046

header subview frame size update

76681bb

header subview frame corrections

92b899a

remove unused import

cfb237e

coado marked this pull request as draft November 14, 2024 14:31

coado added 2 commits November 15, 2024 12:35

flattened header subviews

83fb6f4

iOS frame metrics

5d6716d

coado added 9 commits November 19, 2024 09:33

flattening

b3ebaf5

fix Android

b9e0124

fix layout iOS subviews

aad021a

header config and subviews position

43ddcb2

fix not completed layout

eca6580

fix subview hit test bug

0823b03

hit testing flattened views on iOS

61f2f9d

sending subviews frames on fabric

3b5ee9d

merge

f07b8de

coado added 8 commits November 28, 2024 10:49

fix Android build paper

d6e1597

FabricEnabledHeaderSubviewGroup empty line

723e8a9

remove RNSScreenStackHeaderConfigShadowNode include

b71e62e

remove unused origin from header config state

8541afc

caching header config size, iOS

396abd4

_lastSize recycle

9799d47

remove sending top inset, paper

f7bfbd7

fix Android build

c71ce45

coado marked this pull request as ready for review November 28, 2024 13:14

alecgorge added a commit to RelistenNet/relisten-mobile that referenced this pull request Dec 23, 2024

fix header buttons not working on android

d090a77

workaround from software-mansion/react-native-screens#2219 (comment) software-mansion/react-native-screens#2466

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: clicking on Pressable located in screen header #2466

fix: clicking on Pressable located in screen header #2466

coado commented Oct 29, 2024 •

edited by kkafar

Loading

alduzy commented Oct 30, 2024

coado commented Oct 31, 2024

coado commented Oct 31, 2024

jiroscripts commented Nov 12, 2024

coado commented Nov 13, 2024

kkafar commented Nov 13, 2024

acrabb commented Nov 18, 2024

jiroscripts commented Dec 3, 2024

jiroscripts commented Dec 18, 2024

kkafar commented Dec 20, 2024

sadikyalcin commented Dec 20, 2024

kkafar commented Dec 20, 2024

sadikyalcin commented Dec 20, 2024

nikodunk commented Dec 22, 2024 •

edited

Loading

satya164 commented Dec 24, 2024

fix: clicking on Pressable located in screen header #2466

Are you sure you want to change the base?

fix: clicking on Pressable located in screen header #2466

Conversation

coado commented Oct 29, 2024 • edited by kkafar Loading

Description

Changes

Screenshots / GIFs

Before

After

Test code and steps to reproduce

alduzy commented Oct 30, 2024

coado commented Oct 31, 2024

coado commented Oct 31, 2024

jiroscripts commented Nov 12, 2024

coado commented Nov 13, 2024

kkafar commented Nov 13, 2024

acrabb commented Nov 18, 2024

jiroscripts commented Dec 3, 2024

jiroscripts commented Dec 18, 2024

kkafar commented Dec 20, 2024

sadikyalcin commented Dec 20, 2024

kkafar commented Dec 20, 2024

sadikyalcin commented Dec 20, 2024

nikodunk commented Dec 22, 2024 • edited Loading

satya164 commented Dec 24, 2024

coado commented Oct 29, 2024 •

edited by kkafar

Loading

nikodunk commented Dec 22, 2024 •

edited

Loading